fix(tests): stop webhook render test comparing capped flow-run counts - #10285
Closed
saltas888 wants to merge 1 commit into
Closed
fix(tests): stop webhook render test comparing capped flow-run counts#10285saltas888 wants to merge 1 commit into
saltas888 wants to merge 1 commit into
Conversation
test_branchless_event_triggers_webhook_process counted the deployment's flow runs before and after posting the event and asserted the count grew. The read was unsorted and unlimited, so Prefect returned its default 200-row page: once the webhook-process deployment accumulated 200 runs in a session, both counts pinned at 200 and the assertion could never pass, failing the job regardless of whether the render worked. Read the run ids sorted newest-first and assert a new id appeared, so a full page no longer masks the run. This mirrors the approach already documented for webhook-send runs in the package conftest.
Contributor
There was a problem hiding this comment.
No issues found across 1 file
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Test-only fix for a flaky assertion that compared capped flow-run counts; now reads sorted IDs and checks for new run IDs, with no production behavior change.
Re-trigger cubic
Contributor
|
This should have been fixed in #10332 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
backend/tests/functional/webhook/test_render.py::TestWebhookRender::test_branchless_event_triggers_webhook_processfails intermittently in CI, and once it starts failing in a given job it can never pass.The test counted the webhook-process deployment's flow runs before and after posting the event:
That read is unsorted and passes no
limit, so the Prefect server returns its default page —server.api.default_limit, which is 200. The Prefect server is session-scoped and accumulates webhook-process runs across the whole functional suite, so once the deployment has 200+ runs bothlen()calls pin at exactly 200 andruns_after > runs_beforecan never hold, regardless of whether the render under test worked.The failure output shows precisely that — both sides of the comparison at 200:
This is not a regression from any recent change: the file is byte-identical on
stable,developandrelease-1.11, and the failure depends only on how many runs earlier tests left behind — which is why it presents as flakiness.The package's own
conftest.pyalready documents this exact hazard for the siblingwebhook-sendflow:webhook-processnever got the same treatment.Fix
Read the run ids sorted newest-first and assert a new id appeared, so a full page no longer masks the new run:
The assertion keeps its original meaning (at least one new run). It deliberately does not tighten to "exactly one": this webhook is created with
event_type="all", so unrelated activity in the session can legitimately trigger additional runs during the polling window — unlike the siblingonly_new_runhelper, whose webhook listens for a single event type.The helper is kept local to the test file rather than moved into
conftest.py, because the sibling helpers it mirrors do not exist in stable's conftest — this keeps the fix applying cleanly as it flows outward.Base branch
Targets
stable: the test file is byte-identical onstable,developandrelease-1.11(same blob), so fixing the oldest affected branch lets it reach the others through the normal syncs rather than needing three separate fixes.Verification
The functional test passes locally against a live stack (
uv run pytest backend/tests/functional/webhook/test_render.py— 1 passed, exit 0). A local run starts from a fresh Prefect server, so this exercises the under-200 path and confirms the fix does not break the passing case; the table below is what covers the failing one.Simulated the cap for both implementations. Below it the old code works, which is why this passed historically; at and above it only the new code detects the run:
Confirmed
server.api.default_limit == 200in the installed Prefect, which is the cap the old code hit.Confirmed against the installed client that
read_flow_runsaccepts bothsortanddeployment_filter, and thatFlowRunSort.EXPECTED_START_TIME_DESCexists.ruff checkandruff format --checkclean on the changed file.No changelog fragment: test-only change with no user-facing behaviour difference.